Skip to content

Use more robust way to get the most recent tag #7

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
NullVoxPopuli wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Use more robust way to get the most recent tag #7

NullVoxPopuli wants to merge 1 commit into from

Conversation

@NullVoxPopuli
Copy link
Contributor

@NullVoxPopuli NullVoxPopuli commented Jan 3, 2024

I've run in to this a lot on several repos setting up release-plan for the first time.

Most recently, on the repo for esyes. There was one prior tag, but git describe --abbrev=0 --tags could not find it, giving this error:

 stdout: 'Error: Command failed with exit code 128: git describe --abbrev=0 --tags\n' +
    "fatal: No tags can describe 'c37611084aa5f41f1ab7a357517e596dc5603cfd'.\n" +
    'Try --always, or create some tags.\n' +
    '    at makeError (/home/nvp/Development/OpenSource/starbeam/esyes/node_modules/.pnpm/execa@5.1.1/node_modules/execa/lib/error.js:60:11)\n' +
    '    at module.exports.sync (/home/nvp/Development/OpenSource/starbeam/esyes/node_modules/.pnpm/execa@5.1.1/node_modules/execa/index.js:194:17)\n' +
    '    at Object.lastTag (/home/nvp/Development/OpenSource/starbeam/esyes/node_modules/.pnpm/@ef4+lerna-changelog@2.0.0/node_modules/@ef4/lerna-changelog/lib/git.js:31:18)\n' +

I've found that this proposed technique for getting the most recent tag works better.
I've tested it across a variety of repos:

❯ git describe --tags `git rev-list --tags --max-count=1`
ember-primitives@0.10.1

❯ git describe --tags `git rev-list --tags --max-count=1`
v3.4.3-@embroider/compat

❯ git describe --tags `git rev-list --tags --max-count=1`
ember-resources@6.4.2

❯ git describe --tags `git rev-list --tags --max-count=1`
ember-repl@3.0.0-beta.7

when no tags are found, we get a non-zero exit:

❯ git describe --tags `git rev-list --tags --max-count=1`
fatal: No names found, cannot describe anything.

❯ echo $?
128

This was referenced Jan 3, 2024
Open
Merged
mansona
mansona requested changes Feb 6, 2024
View reviewed changes
Copy link
Member

@mansona mansona left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So first of all, I like the direction of this work. I recently was trying to convince someone to use release-plan on a new project and the initial experience wasn't great so I want to improve it for sure.

I'm a little bit skeptical that this is the right behaviour though 🤔 you say that this is trying to get the "most recent tag" but that's not always what you want. You want the most recent tag on your branch so that you can use release-tag to deploy legacy versions of things. Do you happen to know if that is what this does?

I also feel very conflicted about merging such a fundamental change without having any tests. I know that might be a bit hard to do since we need a real git repo to do verify the nuances that I'm asking about, but we really need to put our heads together and figure out a way to test this before we merge 😞

@gossi
Copy link
Contributor

gossi commented Aug 2, 2024

I see this now as well for me, but it is a different story for why it appears.

On my local machine I can do everything manually and it works. On GH Actions, I see the error. Here is the most recent one: hokulea/hokulea#239

All of that began to happen, when I switched away from volta, here is my PR: hokulea/hokulea#219 (before that PR, release-plan was working as expected).

So some notion of context/environment? Maybe git version or so is different? (I haven't checked that)

@gossi
Copy link
Contributor

gossi commented Aug 3, 2024

Sigh I fixed it: hokulea/hokulea#244

I adjusted the release script for me and broke it... maybe others did so, too?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mansona mansona mansona requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@NullVoxPopuli @gossi @mansona